Skip to content

chore: Rewrite in Python3#157

Closed
ankitdhall wants to merge 2 commits intomasterfrom
ankit/rewrite_py
Closed

chore: Rewrite in Python3#157
ankitdhall wants to merge 2 commits intomasterfrom
ankit/rewrite_py

Conversation

@ankitdhall
Copy link
Copy Markdown
Owner

@ankitdhall ankitdhall commented Oct 16, 2025

Summary by CodeRabbit

  • New Features

    • Added a Python LiDAR–camera calibration package with a ROS node, ArUco marker detection, point-cloud filtering, config-driven parameters, and utilities for estimating rigid transforms and averaging orientations.
  • Tests

    • Added unit tests covering transformation math, quaternion averaging, config parsing, and node callbacks.
  • Documentation

    • Added package README describing the project.
  • Chores

    • Project scaffolding, packaging config, linting config, updated ignore rules, and CI workflow tweak for ROS package names.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Oct 16, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

Adds a new Python ROS package for LiDAR–camera calibration: math utilities for transform/quaternion ops, a ROS node with sensor callbacks, config parsers, unit tests, packaging/linting and CI workflow tweak, plus a comprehensive .gitignore.

Changes

Cohort / File(s) Summary
Repository & Packaging
/.gitignore, .github/workflows/humble.yml, lidar_camera_calibration_py/pyproject.toml, lidar_camera_calibration_py/package.xml, lidar_camera_calibration_py/README.md, lidar_camera_calibration_py/ruff.toml
Added comprehensive ignore rules; made Velodyne package install ROS-distro-aware in CI; added pyproject metadata and console script, ROS package manifest, README, and Ruff config.
Core Math
lidar_camera_calibration_py/src/calibration_math.py
New SVD-based estimate_transform() for rigid-body alignment and average_quaternions() using eigendecomposition; includes type hints and numpy/scipy usage.
ROS Node & Entry
lidar_camera_calibration_py/src/calibration_node.py, lidar_camera_calibration_py/src/main.py
New LidarCameraCalibrationNode class with image, lidar, and camera_info callbacks (ArUco detection, pointcloud filtering, intrinsic logging) and perform_calibration() calling math utilities; main() initializes the ROS node.
Configuration Utilities
lidar_camera_calibration_py/src/config_utils.py
New parsers parse_config_file() and parse_marker_coordinates() for line‑based config and marker files returning structured dicts/lists.
Tests
lidar_camera_calibration_py/tests/test_calibration_math.py, lidar_camera_calibration_py/tests/test_calibration_node.py, lidar_camera_calibration_py/tests/test_config_utils.py
Added unit tests for math routines (identity/translation cases, quaternion averaging), node instantiation and callbacks (with mocked ROS dependencies), and config parsing (valid, missing, malformed cases).

Sequence Diagram

sequenceDiagram
    participant ROS as ROS Master
    participant Node as LidarCameraCalibrationNode
    participant Math as calibration_math

    ROS->>Node: publish Image
    Node->>Node: convert Image -> CV2
    Node->>Node: detect ArUco markers
    Node-->>ROS: log / (marker IDs)

    ROS->>Node: publish PointCloud2
    Node->>Node: convert -> XYZ points
    Node->>Node: apply spatial & intensity filters
    Node-->>ROS: log (filtered count)

    ROS->>Node: publish CameraInfo
    Node-->>ROS: log (intrinsics)

    Node->>Node: perform_calibration(camera_pts, lidar_pts)
    rect rgb(230,245,255)
      Node->>Math: estimate_transform(camera_pts, lidar_pts)
      Math->>Math: center points, cross-covariance, SVD
      Math-->>Node: (R_est, t_est)
    end
    Node-->>ROS: log (R_est, t_est)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 A hop through matrices, quaternions in tow,

ArUco eyes blink where point clouds flow,
SVD hums softly, aligning the view,
Nodes spin and listen — the data feels new,
Tests guard the burrow, tidy and bright. 🎉

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title “chore: Rewrite in Python3” succinctly captures the primary change of overhauling the codebase for Python 3 without extraneous detail, making it clear to reviewers that the pull request focuses on a language upgrade.
Docstring Coverage ✅ Passed Docstring coverage is 95.24% which is sufficient. The required threshold is 80.00%.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ebcef7e and 89eaee7.

📒 Files selected for processing (1)
  • .github/workflows/humble.yml (1 hunks)

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 8

🧹 Nitpick comments (2)
.gitignore (1)

29-31: Clarify intent for ignored directories.
If the goal is to ignore the wandb and modal_scripts directories, prefer adding trailing slashes (wandb/, modal_scripts/) so we don’t accidentally exclude files that legitimately belong in the repo (e.g., modal_scripts.py).

lidar_camera_calibration_py/pyproject.toml (1)

11-16: Move pytest to test extras instead of runtime deps.

pytest is only needed for running the test suite; shipping it as a required runtime dependency bloats installations and complicates downstream packaging. Please move it into an optional/test extras group.

 dependencies = [
-    "pytest",
     "opencv-python",
     "numpy",
     "scipy",
 ]
+
+[project.optional-dependencies]
+test = [
+    "pytest",
+]
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b8e70cb and ebcef7e.

📒 Files selected for processing (12)
  • .gitignore (1 hunks)
  • lidar_camera_calibration_py/README.md (1 hunks)
  • lidar_camera_calibration_py/package.xml (1 hunks)
  • lidar_camera_calibration_py/pyproject.toml (1 hunks)
  • lidar_camera_calibration_py/ruff.toml (1 hunks)
  • lidar_camera_calibration_py/src/calibration_math.py (1 hunks)
  • lidar_camera_calibration_py/src/calibration_node.py (1 hunks)
  • lidar_camera_calibration_py/src/config_utils.py (1 hunks)
  • lidar_camera_calibration_py/src/main.py (1 hunks)
  • lidar_camera_calibration_py/tests/test_calibration_math.py (1 hunks)
  • lidar_camera_calibration_py/tests/test_calibration_node.py (1 hunks)
  • lidar_camera_calibration_py/tests/test_config_utils.py (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
lidar_camera_calibration_py/tests/test_config_utils.py (1)
lidar_camera_calibration_py/src/config_utils.py (2)
  • parse_config_file (4-30)
  • parse_marker_coordinates (32-55)
lidar_camera_calibration_py/src/calibration_node.py (2)
lidar_camera_calibration_py/src/config_utils.py (2)
  • parse_config_file (4-30)
  • parse_marker_coordinates (32-55)
lidar_camera_calibration_py/src/calibration_math.py (1)
  • estimate_transform (6-33)
lidar_camera_calibration_py/tests/test_calibration_node.py (1)
lidar_camera_calibration_py/src/calibration_node.py (4)
  • LidarCameraCalibrationNode (11-92)
  • image_callback (28-45)
  • lidar_callback (47-62)
  • camera_info_callback (64-72)
lidar_camera_calibration_py/tests/test_calibration_math.py (1)
lidar_camera_calibration_py/src/calibration_math.py (2)
  • estimate_transform (6-33)
  • average_quaternions (35-49)
🪛 LanguageTool
lidar_camera_calibration_py/README.md

[grammar] ~1-~1: There might be a mistake here.
Context: ...hon3 version of LiDAR-Camera Calibration # This package will contain the Python imp...

(QB_NEW_EN)

🪛 Ruff (0.14.0)
lidar_camera_calibration_py/tests/test_config_utils.py

46-46: Do not assert blind exception: Exception

(B017)

lidar_camera_calibration_py/src/calibration_math.py

27-27: Unpacked variable S is never used

Prefix it with an underscore or any other dummy variable pattern

(RUF059)

lidar_camera_calibration_py/src/config_utils.py

14-14: Unnecessary mode argument

Remove mode argument

(UP015)


41-41: Undefined name List

(F821)


42-42: Unnecessary mode argument

Remove mode argument

(UP015)

@ankitdhall ankitdhall closed this Oct 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant